Skip to content

Implementation of distinctBy for DurationSumFilter/-Select#3839

Merged
awildturtok merged 3 commits intodevelopfrom
feature/distinct-duration-sum
Feb 25, 2026
Merged

Implementation of distinctBy for DurationSumFilter/-Select#3839
awildturtok merged 3 commits intodevelopfrom
feature/distinct-duration-sum

Conversation

@awildturtok
Copy link
Collaborator

No description provided.

@awildturtok awildturtok requested a review from thoniTUB as a code owner February 3, 2026 15:11
Comment on lines +39 to +45
@Valid
@NotNull
private ColumnId column;

@Valid
@Nullable
private List<ColumnId> distinctBy;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

können wir die final machen ?

@Override
@Valid
@NotNull
private ColumnId column;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private ColumnId column;
private ColumnId dateRangeColumn;

Comment on lines +69 to +71
if (distinctBy != null) {
aggregator = new DistinctValuesWrapperAggregator<>(aggregator, distinctBy.stream().map(ColumnId::resolve).toList());
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was passiert in dem Fall wenn distinctBy = []

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

habs abgefangen, guter Punkt das ist natürlich quatsch


@Override
public SelectConverter<DurationSumSelect> createConverter() {
//TODO apply distinctBy (though needs to be done once other branches
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hier ist noch ein TODO und es ist abgeschnitten

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ups, aber der Todo reisst es schon an, es hängt von einem anderen PR ab. Die aktuelle implementierung von DurationSum ist falsch.

Comment on lines +1 to +6
result,dates,concept select
1,{2010-01-01/2010-01-01},1
3,{2010-01-01/2010-01-01},
4,{2010-01-01/2010-01-02},1
5,{2010-01-01/2010-01-02},2
6,{-∞/2010-01-01}, No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow das sieht irgendwie gefährlich und unintuitiv aus. Das Ergebnis hängt von der Sortierung der Zeilen ab 👀

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naja, es geht am Ende nicht anders. Es geht ja darum, dass bestimmte Rezepte oä nur einmal gezählt werden. Aber ist definitiv wert in der Doku zu erwähnen, guter Punkt.

Copy link
Collaborator

@thoniTUB thoniTUB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doku noch ergänzen

@awildturtok awildturtok merged commit 7c81599 into develop Feb 25, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants